fix(external/sdk): propagate mapToStruct errors instead of silently dropping outputs#565
Merged
Merged
Conversation
⏱ Benchmark Results✅ No significant performance regressions detected. benchstat comparison (baseline → PR)
|
…rkflow#537) Agent-Logs-Url: https://github.com/GoCodeAlone/workflow/sessions/a7196528-aeb7-46d0-80f5-6ba3bdbfd93e Co-authored-by: intel352 <77607+intel352@users.noreply.github.com>
Copilot
AI
changed the title
[WIP] Fix silent drop of Outputs on structpb.NewStruct errors
fix(external/sdk): propagate mapToStruct errors instead of silently dropping outputs
May 6, 2026
intel352
marked this pull request as ready for review
May 6, 2026 06:00
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the external plugin SDK gRPC server to stop silently dropping step/service outputs when structpb.NewStruct fails, and instead propagates encoding errors back to the caller via the response Error field. This aligns the SDK’s behavior with the already-fixed plugin/external/convert.go path and prevents “empty output means no changes” misinterpretations at the gRPC boundary.
Changes:
- Update
mapToStructin the SDK gRPC server to return(*structpb.Struct, error)and propagatestructpb.NewStructfailures. - Handle encoding errors in
ExecuteStepandInvokeServicecall sites by returning response-level errors. - Add/adjust SDK tests to validate error propagation and update docs to reflect the new signature.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| plugin/external/sdk/grpc_server.go | Propagates structpb.NewStruct errors and surfaces them via response Error fields at gRPC boundaries. |
| plugin/external/sdk/grpc_server_test.go | Adds tests/helpers to ensure encoding failures are not silently dropped and updates existing tests for new signature. |
| docs/PLUGIN_ARCHITECTURE.md | Updates documented mapToStruct signature to include error return. |
| }) | ||
| if err != nil { | ||
| t.Fatalf("CreateModule rpc error: %v", err) | ||
| } |
Contributor
|
@copilot apply changes based on the comments in this thread |
…stMessage formatting Agent-Logs-Url: https://github.com/GoCodeAlone/workflow/sessions/2f8f93e8-62d7-4d38-870c-0e876ea1f068 Co-authored-by: intel352 <77607+intel352@users.noreply.github.com>
Contributor
Author
Done in e16166b:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
plugin/external/sdk/grpc_server.go::mapToStructsuppressedstructpb.NewStructerrors vias, _ = structpb.NewStruct(m), returning nil/empty instead of surfacing the failure. AnyOutputsor service result containing a structpb-incompatible type (chan,func, unserializable struct) would silently vanish at the gRPC boundary — downstream consumers saw an empty map and treated it as "no changes."plugin/external/convert.gowas already fixed with the correct signature; this PR applies the same fix to the SDK's parallel copy.Changes
sdk/grpc_server.go—mapToStruct: changed signature to(*structpb.Struct, error); removed_error suppression; wraps withfmt.Errorf("structpb.NewStruct: %w", err)sdk/grpc_server.go— call sites (ExecuteStep,InvokeService×2): handle error and surface it inResponse.Error(consistent with existing//nolint:nilerrerror-in-response-body pattern)sdk/grpc_server_test.go: addedmustMapToStructtest helper for known-good fixture data; updated existing call sites; addedTestMapToStruct_SDK_PropagatesErrorandTestInvokeService_PropagatesOutputEncodingErrordocs/PLUGIN_ARCHITECTURE.md: updated documented signature